feat(sinker): Ship 8 — high-bit 4:2:2 RGBA sinker integration#28
Conversation
There was a problem hiding this comment.
Pull request overview
Adds packed RGBA (u8) and native-depth RGBA (u16) output wiring for high-bit-depth 4:2:2 formats in MixedSinker, along with targeted regression tests, and updates a compile-fail doc example to keep it valid after the new wiring.
Changes:
- Implement
with_rgba/set_rgbaandwith_rgba_u16/set_rgba_u16for multiple high-bit 4:2:2 sinker formats, including Strategy-A “RGB then expand to RGBA” behavior. - Add tests validating gray conversion, opaque alpha, Strategy-A equivalence (RGB-only vs RGB+RGBA), and short-buffer errors for selected 4:2:2 formats.
- Update the planar 8-bit sinker doc compile-fail snippet to use a still-not-wired format for RGBA.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/sinker/mixed/tests.rs | Adds new tests for high-bit 4:2:2 RGBA/u16-RGBA wiring and error paths. |
| src/sinker/mixed/subsampled_4_2_2_high_bit.rs | Wires RGBA outputs (u8/u16) for high-bit 4:2:2 planar and P21x formats, including Strategy-A expand paths. |
| src/sinker/mixed/planar_8bit.rs | Updates a compile_fail doc example to remain a valid negative case now that Yuv422p10 supports RGBA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ---- Ship 8 PR 5d: high-bit 4:2:2 RGBA wiring ------------------------- | ||
| // | ||
| // Strategy A combine for the eight 4:2:2 high-bit sinker formats wired | ||
| // in the 4:2:2 high-bit file. Mirrors the 4:2:0 PR #26 test suite; | ||
| // covers Yuv422p10 (planar BITS-generic), Yuv422p16 (planar 16-bit | ||
| // dedicated kernel), and P210 (semi-planar BITS-generic) — the row | ||
| // layer is exhaustively tested elsewhere. |
There was a problem hiding this comment.
This new test tranche is described as covering the 4:2:2 high-bit RGBA wiring for eight formats, but it only exercises Yuv422p10/Yuv422p16/P210. Please consider adding at least lightweight smoke tests for the remaining newly-wired formats (e.g., Yuv422p9/12/14 and P212/P216) to validate alpha fill semantics across BITS (4095/16383/0xFFFF) and catch format-specific wiring regressions.
There was a problem hiding this comment.
Defensible suggestion, but deferring this one. Two reasons:
-
Precedent from PR Ship 8 Tranche 5b: high-bit 4:2:0 RGBA u16 SIMD + sinker integration #26 (Tranche 5b): the 4:2:0 sinker tests added there only smoke-tested
Yuv420p10/P010/Yuv420p16—Yuv420p9/Yuv420p12/Yuv420p14/P012weren't covered either. The 4:2:2 PR mirrors that gap exactly, so closing it here would create per-PR-asymmetric coverage. -
Row-layer covers per-BITS alpha exhaustively: the per-backend RGBA equivalence tests added in PR Ship 8 Tranche 5a: high-bit 4:2:0 RGBA u8 SIMD #25/Ship 8 Tranche 5b: high-bit 4:2:0 RGBA u16 SIMD + sinker integration #26 already prove alpha =
(1 << BITS) - 1(BITS=9/10/12/14) and0xFFFF(BITS=16) across all 5 SIMD backends + scalar reference. A sinker-layer smoke test forYuv422p9etc. would only re-prove that the sinker calls the dispatcher with the right buffer — which the existingYuv422p10/Yuv422p16/P210tests already confirm for representative kernel families (BITS-generic planar, 16-bit dedicated, BITS-generic semi-planar).
Happy to add the per-format smoke tests as a follow-up if the symmetric gap on the 4:2:0 side bothers reviewers — would prefer to do both in a single cleanup PR.
…16-bit kernel The `with_rgba` doc on `MixedSinker<Yuv422p16>` and `MixedSinker<P216>` claimed the 16-bit YUV → 8-bit RGBA path uses "the `BITS = 16` Q15 kernel family", but both formats actually call `yuv420p16_to_rgba_row` / `p016_to_rgba_row` — the **dedicated 16-bit kernel family** (i64 chroma multiply, separate from the BITS-generic Q15 pipeline used at BITS=10/12/14). Aligns the wording with the existing `Yuv420p16` / `P016` doc convention in `subsampled_4_2_0_high_bit.rs` and explicitly contrasts the Q15 pipeline so future readers don't conflate the two kernel families. Addresses Copilot review comments on PR #28. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Wires
with_rgba(u8) +with_rgba_u16(u16) into the 8 high-bit 4:2:2 sinker formats insrc/sinker/mixed/subsampled_4_2_2_high_bit.rsand updates eachPixelSink::processto consume them via Strategy A combine. Pure sinker-layer integration — no new SIMD kernels, no new dispatchers, no row-level changes: every format reuses an existing 4:2:0 high-bit RGBA dispatcher already wired in PR #25 (Tranche 5a) + PR #26 (Tranche 5b).Why no row-level work
4:2:2 has the same per-row chroma shape as 4:2:0 (half-width U/V, one pair per Y pair) — only the vertical subsampling differs. The walker handles row cadence; the row primitive is shared. This kernel-reuse pattern was already established for the existing 4:2:2 high-bit RGB path (each
Yuv422p<N>::processcallsyuv420p<N>_to_rgb_row).Changes
Sinker (
src/sinker/mixed/subsampled_4_2_2_high_bit.rs)8 sinker impl blocks gain
with_rgba/set_rgba/with_rgba_u16/set_rgba_u16— 32 new methods. Each format'sprocess()is restructured to consume the new buffers via Strategy A:Yuv422p9yuv420p9_to_rgba(_u16)_rowYuv422p10yuv420p10_to_rgba(_u16)_rowYuv422p12yuv420p12_to_rgba(_u16)_rowYuv422p14yuv420p14_to_rgba(_u16)_rowYuv422p16yuv420p16_to_rgba(_u16)_rowP210p010_to_rgba(_u16)_rowP212p012_to_rgba(_u16)_rowP216p016_to_rgba(_u16)_rowStrategy A pattern (mirrors the 4:2:0 PR #26 implementation):
rgba_u16-only routes directly through*_to_rgba_u16_row;rgb_u16 + rgba_u16runs the RGB kernel once and fans out viaexpand_rgb_u16_to_rgba_u16_row::<BITS>(cheap per-pixel pad with depth-aware alpha).rgba-only goes direct;rgb + rgba(orhsv + rgba) uses the existing scratch +expand_rgb_to_rgba_rowfan-out.P210/P212/P216previously had noconst BITS: u32declaration in theirprocess()(luma used a hardcoded>> 8); added one per format soexpand_rgb_u16_to_rgba_u16_row::<BITS>can be invoked uniformly.Doc-fail example (
src/sinker/mixed/planar_8bit.rs)The
compile_faildoctest demonstrating "attaching RGBA to a sink that doesn't write it is rejected" had been pointing atYuv422p10after PR #26. After this PR,Yuv422p10does write RGBA — moved the example toYuv444p10(4:4:4 high-bit, still genuinely lackswith_rgbauntil the next PR adds it).Tests (
src/sinker/mixed/tests.rs)+8 new sinker tests (test count 499 → 507):
Yuv422p10u8 RGBA gray-to-gray with opaque alpha (0xFF)Yuv422p10u16 RGBA gray-to-gray with alpha = 1023Yuv422p10Strategy A u8:with_rgb + with_rgbabyte-identical RGBYuv422p10Strategy A u16:with_rgb_u16 + with_rgba_u16byte-identical RGBYuv422p10RgbaBufferTooShorterrYuv422p10RgbaU16BufferTooShorterrP210u16 RGBA gray-to-gray (covers BITS-generic Pn semi-planar)Yuv422p16u16 RGBA gray-to-gray (covers 16-bit dedicated kernel)Explicitly out of scope
Yuv440p10/Yuv440p12(also live in this file) reuse 4:4:4 high-bit dispatchers — those don't have RGBA wired yet. Deferred to the next PR alongside the 4:4:4 high-bit RGBA work.Test plan
cargo test --lib: 507 pass, 0 fail (was 499; +8 new tests, all in scope)cargo check --tests --libclean across host (aarch64-darwin), x86_64-unknown-freebsd, wasm32-unknown-unknownRUSTFLAGS=\"-Dwarnings\" cargo clippy --lib --testscleancargo test --docclean (the updatedYuv444p10compile_failexample correctly fails to compile)Codex adversarial review
Verdict: approve. No material findings. (review log)
🤖 Generated with Claude Code